Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add MAVLink message actions #1436

Merged

Conversation

rafaellehmkuhl
Copy link
Member

@rafaellehmkuhl rafaellehmkuhl commented Nov 5, 2024

Any message is accepted as long as it's a valid MAVLink message.

For COMMAND_LONG and COMMAND_INT messages, the message fields are pre-defined in the UI. For all other messages, the user should specify the JSON message config manually.

I intend to do a proper automatic parser for all messages soon.

image

Fix #825
Fix #826

Sometimes anything is something.
Copy link
Contributor

@ArturoManzoli ArturoManzoli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still reviewing the code and functionalities, but for now, here are some UI observations:
The form is a little bit crammed and the confirm button color needs more contrast.
Try changing the following:

1- Change the confirm button color to white and the others to white but with 80% opacity.
2- Increase the gap-y to let some breeze flow through the form elements :)
3- I think there is no need to repeat Param2 on the box and Parameter 2 on the bottom. Probably only on the box will be fine.

Also, the title 'Message configuration' should be centered on the dialog.

image

@rafaellehmkuhl
Copy link
Member Author

rafaellehmkuhl commented Nov 6, 2024

Still reviewing the code and functionalities, but for now, here are some UI observations: The form is a little bit crammed and the confirm button color needs more contrast. Try changing the following:

1- Change the confirm button color to white and the others to white but with 80% opacity. 2- Increase the gap-y to let some breeze flow through the form elements :) 3- I think there is no need to repeat Param2 on the box and Parameter 2 on the bottom. Probably only on the box will be fine.

Also, the title 'Message configuration' should be centered on the dialog.

Thanks for the tips! Will add them already and push after the rest of the review. Added:

image

Any message is accepted as long as it's a valid MAVLink message.

For COMMAND_LONG and COMMAND_INT messages, the message fields are pre-defined in the UI. For all other messages, the user should specify the JSON message config manually.
@rafaellehmkuhl rafaellehmkuhl force-pushed the add-mavlink-message-actions branch from 0362b51 to 47eaa28 Compare November 6, 2024 17:10
Copy link
Contributor

@ArturoManzoli ArturoManzoli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice feature!

@ArturoManzoli ArturoManzoli merged commit 5147655 into bluerobotics:master Nov 6, 2024
10 checks passed
@rafaellehmkuhl rafaellehmkuhl deleted the add-mavlink-message-actions branch November 10, 2024 21:38
@ES-Alexander ES-Alexander added the docs-needed Change needs to be documented label Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-needed Change needs to be documented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Joystick: support custom MAVLink commands as actions Joystick: add actions for MAVLink gripper control
3 participants